-
Notifications
You must be signed in to change notification settings - Fork 58
Vehicle rental overlay graphql migration #1440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
…-graphql-migration
…-graphql-migration
…icle-rental-overlay-graphql-migration
…-graphql-migration
…l-overlay-graphql-migration
…-graphql-migration
…-graphql-migration
…tal-overlay-graphql-migration
…-graphql-migration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some more types to the DefaultMap type before I approve?
| const micromobility = rentalVehicles.filter( | ||
| (vehicle) => | ||
| vehicle.vehicleType && | ||
| vehicle.vehicleType?.formFactor !== FormFactor.CAR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will filter out any vehicles where vehicleType is undefined. Is that intentional? Do scooters/bikes never have undefined? Under what circumstance can it be undefined?
You should be able to remove the ? from the second part of the condition since the null check has already happened.
| setLocation: any | ||
| setViewedStop: any | ||
| viewedRouteStops: any | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could define a bunch of these. There are some low hanging fruit like intl -> IntlShape.
Description:
This PR updates the
bikeRentalQueryto use graphQL instead of the deprecated REST endpoint.Note that the updates to OTP-UI found in this PR must be merged to correctly use the response that comes back from graphQL.PR Checklist: